Skip to content
This repository was archived by the owner on Mar 21, 2022. It is now read-only.

integrating PR #340 into latest docker-client#691

Open
grexe wants to merge 8 commits intospotify:masterfrom
grexe:master
Open

integrating PR #340 into latest docker-client#691
grexe wants to merge 8 commits intospotify:masterfrom
grexe:master

Conversation

@grexe
Copy link
Copy Markdown

@grexe grexe commented Mar 20, 2017

this PR builds on PR #340 and brings it up to date with latest additions, e.g. had to extend the ClientFactory to provide the HTTP client based on REST impl, fixed dependency issues for projects including the lib.
Only the Swarm tests fail. Works in Wildfly 10 with Docker 1.12.6 on Ubuntu 16.10 x64.
Please review for integration.

jskjons and others added 8 commits May 6, 2016 08:42
This required:
  - adding a few annotations
  - adding resteasy dependencies
  - adding hackery to make streaming work with RESTEasy
  - switching to Parameterized JUnit tests
  - documentation for how to use it
…L/ProgressStream, fixed merge errors, added needed factory method for HTTP client.
@jwgmeligmeyling
Copy link
Copy Markdown
Contributor

jwgmeligmeyling commented Mar 20, 2017

Glad someone picks up this issue again! But it seems you did some formatting changes too... With 7k +/- line changes its almost impossible to see what has changed and it will be a pain to review. It cases the new merge conflicts too. Can you revert the formatting changes? (As I remember too that the original PR only had about 700 line changes and was pretty easy to read)

@grexe
Copy link
Copy Markdown
Author

grexe commented Mar 20, 2017

sorry for that @jwgmeligmeyling - seems my IntelliJ formatter was a bit over aggressive (using standard 4 spaces indentation etc. though).
Will check, maybe you can compare with ignore-whitespace in the meantime?

@grexe
Copy link
Copy Markdown
Author

grexe commented Mar 20, 2017

PS: didn't test with the JerseyClientProvider yet btw for regressions, but didn't change anything there.

@jwgmeligmeyling
Copy link
Copy Markdown
Contributor

Ah yes, I will review it with ignore-whitespace for now. Going to test against Wildfly 10.1.0.Final as well.

@jwgmeligmeyling
Copy link
Copy Markdown
Contributor

I don't know exactly but I vaguely remember that the formatting here was 2 spaces in Google Code style (for imports too). Can't find that back in the CONTRIBUTING.md anymore though...

@grexe
Copy link
Copy Markdown
Author

grexe commented Mar 28, 2017

found a missing dependency in my PR, will fix and update the PR.
What do you think so far @jwgmeligmeyling ?

@bitsofinfo
Copy link
Copy Markdown

+1

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants